Skip to content

fix(webapp): allow JWT auth on POST /api/v1/sessions#3474

Merged
ericallam merged 2 commits intomainfrom
fix/api-v1-sessions-allow-jwt
Apr 30, 2026
Merged

fix(webapp): allow JWT auth on POST /api/v1/sessions#3474
ericallam merged 2 commits intomainfrom
fix/api-v1-sessions-allow-jwt

Conversation

@ericallam
Copy link
Copy Markdown
Member

Summary

POST /api/v1/sessions was secret-key-only because the customer browser flow runs through chat.createStartSessionAction (server-side, holds the secret key). But the cli-v3 MCP start_agent_chat tool is itself a server-side surface — developer's CLI/IDE acting as their own server — and only holds a JWT minted from the user's PAT. Without JWT support on this route the entire MCP agent toolkit (start_agent_chat, send_agent_message, close_agent_chat) is blocked at session creation.

Add allowJWT: true plus an authorization block requiring the write:sessions (or admin) super-scope.

Why a wildcard sessions resource

Resource scoping by taskIdentifier isn't possible at auth-resolve time — action routes don't pass body to the resource callback, and the task name only lives in the body. So the resource is sessions: "*" and the super-scope does the actual gating. The JWT-issuer (cli-v3 MCP, customer servers wrapping their own auth helpers, etc.) decides which scopes to mint, which is where per-task narrowing lives.

Test plan

  • Verified end-to-end against local: mcp__trigger__start_agent_chatsend_agent_message("pong")send_agent_message("echo")close_agent_chat all succeed. Two assistant turns reuse the same runId (continuation in the idle window).
  • Browser-mediated chat.createStartSessionAction flow continues to work unchanged (still uses secret-key path under the hood).
  • Loader (GET) and other session routes — unchanged, no scope drift.

Notes

This unblocks T17 in the ai-chat e2e smoke catalog (which lives in the feature branch's skill catalog, not this repo). Pairs with the cli-v3 MCP fix on the feature branch (feat: AI SDK custom useChat transport & chat.task harness, PR #3173) — that PR's agentChat.ts change makes the call shape correct (taskIdentifier + triggerConfig); this PR opens the door for the JWT to actually pass.

The route was secret-key-only by design ("Customer's server owns session creation"). That holds for the customer browser path — `chat.createStartSessionAction` runs server-side and authorizes there. But the cli-v3 MCP `start_agent_chat` tool is itself a server-side surface (developer's CLI/IDE acting as their own server) and only holds a JWT minted from the user's PAT. Without JWT support here it can't create sessions, blocking the entire MCP agent toolkit.

Add `allowJWT: true` and an `authorization` block requiring the `write:sessions` (or `admin`) super-scope. Resource scoping by `taskIdentifier` isn't possible at auth-resolve time — action routes don't pass `body` to the resource callback, and the task name only lives in the body — so the resource is a `sessions` wildcard and the super-scope does the gating. The JWT-issuer (cli-v3 MCP, customer servers wrapping a wider auth helper, etc.) decides which scopes to mint, which is where per-task narrowing lives.

Verified end-to-end: `mcp__trigger__start_agent_chat` → `send_agent_message("pong")` → `send_agent_message("echo")` → `close_agent_chat` all succeed against local. Two assistant turns reuse the same runId (continuation in the idle window).
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 29, 2026

⚠️ No Changeset found

Latest commit: c438fdf

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 29, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 498c3faf-226f-4623-bf20-0b2df9de5a38

📥 Commits

Reviewing files that changed from the base of the PR and between 51b0fcd and c438fdf.

📒 Files selected for processing (1)
  • apps/webapp/app/routes/api.v1.sessions.ts
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: typecheck / typecheck
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead

Files:

  • apps/webapp/app/routes/api.v1.sessions.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use zod for validation in packages/core and apps/webapp

Files:

  • apps/webapp/app/routes/api.v1.sessions.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use function declarations instead of default exports

Add crumbs as you write code using // @Crumbs comments or `// `#region` `@crumbs blocks. These are temporary debug instrumentation and must be stripped using agentcrumbs strip before merge.

Files:

  • apps/webapp/app/routes/api.v1.sessions.ts
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)

**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries

Files:

  • apps/webapp/app/routes/api.v1.sessions.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}

📄 CodeRabbit inference engine (AGENTS.md)

Format code using Prettier before committing

Files:

  • apps/webapp/app/routes/api.v1.sessions.ts
**/*.ts{,x}

📄 CodeRabbit inference engine (CLAUDE.md)

Always import from @trigger.dev/sdk when writing Trigger.dev tasks. Never use @trigger.dev/sdk/v3 or deprecated client.defineJob.

Files:

  • apps/webapp/app/routes/api.v1.sessions.ts
apps/webapp/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

apps/webapp/**/*.{ts,tsx}: Access environment variables through the env export of env.server.ts instead of directly accessing process.env
Use subpath exports from @trigger.dev/core package instead of importing from the root @trigger.dev/core path

Use named constants for sentinel/placeholder values (e.g. const UNSET_VALUE = '__unset__') instead of raw string literals scattered across comparisons

Files:

  • apps/webapp/app/routes/api.v1.sessions.ts
🧠 Learnings (14)
📚 Learning: 2026-03-13T13:42:59.104Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3213
File: apps/webapp/app/routes/admin.api.v1.llm-models.$modelId.ts:40-43
Timestamp: 2026-03-13T13:42:59.104Z
Learning: In `apps/webapp/app/routes/admin.api.v1.llm-models.$modelId.ts` and `apps/webapp/app/routes/admin.api.v1.llm-models.ts`, the `startDate` field in `UpdateModelSchema` and `CreateModelSchema` intentionally uses `z.string().optional()` (or `.nullable().optional()`) without strict ISO datetime validation. Invalid date strings are rejected at the Prisma/DB layer. This is acceptable because these are admin-only API routes protected by Personal Access Token (PAT) authentication and are not user-facing.

Applied to files:

  • apps/webapp/app/routes/api.v1.sessions.ts
📚 Learning: 2026-04-13T21:44:00.032Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3368
File: apps/webapp/app/services/taskIdentifierRegistry.server.ts:24-67
Timestamp: 2026-04-13T21:44:00.032Z
Learning: In `apps/webapp/app/services/taskIdentifierRegistry.server.ts`, the sequential upsert/updateMany/findMany writes in `syncTaskIdentifiers` are intentionally NOT wrapped in a Prisma transaction. This function runs only during deployment-change events (low-concurrency path), and any partial `isInLatestDeployment` state is acceptable because it self-corrects on the next deployment. Do not flag this as a missing-transaction/atomicity issue in future reviews.

Applied to files:

  • apps/webapp/app/routes/api.v1.sessions.ts
📚 Learning: 2026-02-25T17:28:20.456Z
Learnt from: isshaddad
Repo: triggerdotdev/trigger.dev PR: 3130
File: docs/v3-openapi.yaml:3134-3135
Timestamp: 2026-02-25T17:28:20.456Z
Learning: In the Trigger.dev codebase, the `publicAccessToken` returned by the SDK's `wait.createToken()` method is not part of the HTTP response body from `POST /api/v1/waitpoints/tokens`. The server returns only `{ id, isCached, url }`. The SDK's `prepareData` hook generates the JWT client-side from the `x-trigger-jwt-claims` response header after the HTTP call completes. The OpenAPI spec correctly documents only the HTTP response body, not SDK transformations.
<!-- [/add_learning]

Applied to files:

  • apps/webapp/app/routes/api.v1.sessions.ts
📚 Learning: 2026-04-20T15:05:59.661Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3417
File: apps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.ts:20-31
Timestamp: 2026-04-20T15:05:59.661Z
Learning: In `apps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.ts`, the `MAX_APPEND_BODY_BYTES` cap is intentionally set to `1024 * 512` (512 KiB). The maintainer explicitly decided against lowering it to 128 KiB: the all-quotes worst-case JSON-escaping expansion that could exceed S2's 1 MiB per-record limit is considered pathological and not representative of real-world payloads (chat tokens, tool-call JSON, structured data). If overflow becomes a problem in practice, the preferred mitigation is an encoded-size guard inside `appendPart` itself. Do not flag this cap as a potential S2 overflow issue in future reviews.

Applied to files:

  • apps/webapp/app/routes/api.v1.sessions.ts
📚 Learning: 2026-04-16T13:45:22.317Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3368
File: apps/webapp/test/engine/taskIdentifierRegistry.test.ts:3-19
Timestamp: 2026-04-16T13:45:22.317Z
Learning: In `apps/webapp/test/engine/taskIdentifierRegistry.test.ts`, the `vi.mock` calls for `~/services/taskIdentifierCache.server` (stubbing `getTaskIdentifiersFromCache` and `populateTaskIdentifierCache`), `~/models/task.server` (stubbing `getAllTaskIdentifiers`), and `~/db.server` (stubbing `prisma` and `$replica`) are intentional. The suite uses real Postgres via testcontainers for all `TaskIdentifier` DB operations, but isolates the Redis cache layer and legacy query fallback as separate concerns not exercised in this test file. Do not flag these mocks as violations of the no-mocks policy in future reviews.

Applied to files:

  • apps/webapp/app/routes/api.v1.sessions.ts
📚 Learning: 2026-04-20T15:06:11.054Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3417
File: apps/webapp/app/routes/realtime.v1.streams.$runId.$target.$streamId.append.ts:16-26
Timestamp: 2026-04-20T15:06:11.054Z
Learning: In `apps/webapp/app/routes/realtime.v1.streams.$runId.$target.$streamId.append.ts` and `apps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.ts`, the `MAX_APPEND_BODY_BYTES` cap of 512 KiB (1024 * 512) is intentional even though `appendPart` wraps the body in JSON (which could expand quote-heavy payloads beyond S2's 1 MiB per-record limit). The maintainer considers worst-case quote-heavy payloads pathological and not realistic. If S2 rejections occur in practice, an encoded-size guard will be added inside `appendPart` rather than lowering the raw body cap on every caller. Do not flag this as an issue in future reviews.

Applied to files:

  • apps/webapp/app/routes/api.v1.sessions.ts
📚 Learning: 2026-04-16T14:21:11.115Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3368
File: apps/webapp/app/services/taskIdentifierCache.server.ts:33-39
Timestamp: 2026-04-16T14:21:11.115Z
Learning: In `apps/webapp/app/services/taskIdentifierCache.server.ts`, the `decode()` function intentionally uses a plain `JSON.parse` cast instead of Zod validation. The Redis cache is exclusively written by the internal `populateTaskIdentifierCache` function via the symmetric `encode()` helper — there is no external input path. Any shape mismatch would be a serialization bug to surface explicitly, not untrusted data to filter out. Do not suggest adding Zod validation to the `decode()` function or the `getTaskIdentifiersFromCache` return path in future reviews.

Applied to files:

  • apps/webapp/app/routes/api.v1.sessions.ts
📚 Learning: 2026-03-10T17:56:26.581Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3201
File: apps/webapp/app/v3/services/setSeatsAddOn.server.ts:25-29
Timestamp: 2026-03-10T17:56:26.581Z
Learning: In the `triggerdotdev/trigger.dev` webapp, service classes such as `SetSeatsAddOnService` and `SetBranchesAddOnService` do NOT need to perform their own userId-to-organizationId authorization checks. Auth is enforced at the route layer: `requireUserId(request)` authenticates the user, and the `_app.orgs.$organizationSlug` layout route enforces that the authenticated user is a member of the org. Any `userId` and `organizationId` reaching these services from org-scoped routes are already validated. This is the consistent pattern used across all org-scoped services in the codebase.

Applied to files:

  • apps/webapp/app/routes/api.v1.sessions.ts
📚 Learning: 2026-02-11T16:50:14.167Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardId.widgets.tsx:126-131
Timestamp: 2026-02-11T16:50:14.167Z
Learning: In apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardId.widgets.tsx, MetricsDashboard entities are intentionally scoped to the organization level, not the project level. The dashboard lookup should filter by organizationId only (not projectId), allowing dashboards to be accessed across projects within the same organization. The optional projectId field on MetricsDashboard serves other purposes and should not be used as an authorization constraint.

Applied to files:

  • apps/webapp/app/routes/api.v1.sessions.ts
📚 Learning: 2025-08-14T10:53:54.526Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 2391
File: apps/webapp/app/services/organizationAccessToken.server.ts:50-0
Timestamp: 2025-08-14T10:53:54.526Z
Learning: In the Trigger.dev codebase, token service functions (like revokePersonalAccessToken and revokeOrganizationAccessToken) don't include tenant scoping in their database queries. Instead, authorization and tenant scoping happens at a higher level in the authentication flow (typically in route handlers) before these service functions are called. This is a consistent pattern across both Personal Access Tokens (PATs) and Organization Access Tokens (OATs).

Applied to files:

  • apps/webapp/app/routes/api.v1.sessions.ts
📚 Learning: 2026-03-26T09:02:11.935Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 3274
File: apps/webapp/app/services/runsReplicationService.server.ts:922-924
Timestamp: 2026-03-26T09:02:11.935Z
Learning: In `triggerdotdev/trigger.dev`, `TaskRun.annotations` are always written atomically in one operation that conforms exactly to the `RunAnnotations` schema (from `trigger.dev/core/v3`). Using `RunAnnotations.safeParse` in `#parseAnnotations` (e.g., in `apps/webapp/app/services/runsReplicationService.server.ts`) is intentional and correct — there is no risk of partial/legacy annotation payloads causing schema mismatches, so suggesting a relaxed passthrough schema for this parsing is unnecessary.

Applied to files:

  • apps/webapp/app/routes/api.v1.sessions.ts
📚 Learning: 2026-02-19T18:09:23.944Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3095
File: apps/webapp/app/components/navigation/DashboardDialogs.tsx:47-47
Timestamp: 2026-02-19T18:09:23.944Z
Learning: In the triggerdotdev/trigger.dev codebase, the pattern `isPaying === false` is used consistently to explicitly check for free plan users. This is a project convention that distinguishes between `isPaying === true` (paying), `isPaying === false` (free), and `isPaying === undefined` (no subscription data). Do not suggest changing this to negation pattern.
```
<!-- <review_comment_addressed>

Applied to files:

  • apps/webapp/app/routes/api.v1.sessions.ts
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).

Applied to files:

  • apps/webapp/app/routes/api.v1.sessions.ts
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.

Applied to files:

  • apps/webapp/app/routes/api.v1.sessions.ts
🔇 Additional comments (1)
apps/webapp/app/routes/api.v1.sessions.ts (1)

99-131: JWT authorization narrowing is implemented cleanly here.

The allowJWT enablement plus task-scoped resource callback and write:sessions/admin super-scope fallback matches the intended access model and avoids the resource-type OR pitfall called out in your inline note.


Walkthrough

The session creation POST route was changed to allow JWT authentication (allowJWT: true) in addition to secret-key callers. Route-level authorization now requires super-scopes (write:sessions or admin) and enforces a task-scoped resource constraint derived from the request body (tasks: body.taskIdentifier). Inline documentation was added explaining that the authorization resource callback scopes by taskIdentifier, and warns that adding both session and task resource types weakens per-task narrowing because resource-type checks are ORed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: enabling JWT authentication on the POST /api/v1/sessions endpoint, which is the primary objective of the PR.
Description check ✅ Passed The PR description comprehensively covers the motivation, implementation details, resource-scoping rationale, and test plan. However, one test item (browser-mediated chat.createStartSessionAction) remains unchecked.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/api-v1-sessions-allow-jwt

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

devin-ai-integration[bot]

This comment was marked as resolved.

Devin caught two real issues with the previous resource shape on PR #3474:

1. The "body isn't available at auth-resolve time" claim was wrong. Action-route resource callbacks receive the parsed body as the 4th arg (apiBuilder.server.ts:710). Other routes like api.v1.tasks.batch.ts use it (line 33).
2. The auth check is OR across resource types — listing both `sessions: "*"` and `tasks: body.taskIdentifier` would let a `write:sessions`-only JWT pass for any task, defeating the per-task narrowing.

Replace `() => ({ sessions: "*" })` with `(_, __, ___, body) => ({ tasks: body.taskIdentifier })` and rely on the `write:sessions` / `admin` super-scopes for broad access. A JWT scoped only to `write:tasks:foo` can now only create sessions whose taskIdentifier is `foo`. MCP-style flows that hold `write:sessions` continue to work via the super-scope path.

Verified: MCP `start_agent_chat` → `send_agent_message` → `close_agent_chat` still passes locally; webapp typecheck clean.
@ericallam ericallam merged commit 04b4d85 into main Apr 30, 2026
44 checks passed
@ericallam ericallam deleted the fix/api-v1-sessions-allow-jwt branch April 30, 2026 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants